refactor(sync): make the fork check unavoidable by construction - #235

The consumer was the one place the fork check was still made by convention. It
held a bare `tokio::runtime::Handle` field next to a hand-written `guard()`,
called at five sites, which returned `Ok` when the `Weak` upgrade failed. That
is the shape this whole change exists to remove — opt-in guards and a resource
any method can reach without passing one — rebuilt on a single type. Naming it
"the one thing still enforced by convention" documented the gap rather than
closing it.

The consumer genuinely needs a handle that outlives the runtime: a `Reader` can
still drain what is already buffered after a `detach`, and delivering that data
is behaviour the characterization tests pin. So the handle stays — but it moves
into `RuntimeRef` in `runtime.rs`, where both fields are private to that module.
`consumer.rs` can no longer obtain a handle except through `RuntimeRef::enter`,
which checks first, so the blocking reads are checked by construction exactly
as the publish path is.

`try_get` still calls the check explicitly, and that is not a gap being glossed:
it touches no runtime resource at all, so there is nothing to gate. `get_latest`
and `get_latest_with_timeout` look like the same case but route through
`get`/`get_with_timeout`, which are gated. One explicit check, not five.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

…check

`try_get` was the one remaining place the fork check was a call rather than a
consequence. The justification given for that — it touches no runtime resource,
so there is nothing to gate — was wrong. It touches the `Reader`, and the
`Reader` is exactly the thing to gate: something with no resource behind a
checked accessor is something whose check is easy to leave out, which is the
defect this design exists to remove.

The reader now lives in `Guarded<Reader<T>>`, whose value is private to
`runtime.rs`. `consumer.rs` reaches it through `get()` (checked) or `enter()`
(checked, and yields a Tokio handle for the reads that block). `SyncConsumer`
is down to a single field and no longer names the runtime at all, so there is
no longer a check to remember or forget: every read passes one because it
cannot reach the buffer otherwise.

`get_latest` and `get_latest_with_timeout` lose their explicit checks with
nothing lost — both route through `get`/`get_with_timeout`/`try_get`, all of
which are now gated at the point of access.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

`db_unchecked` existed for one caller — `AimDbHandle::consumer`, which checked
and then subscribed — and was documented as "not a hole in the guarantee"
because its single use sat next to a check. That is the argument every such
accessor comes with, and it only holds until someone adds a second caller.
`consumer()` now subscribes through `db()`, which checks, so the accessor has
no reason to exist and is gone. No unchecked accessors remain in the crate.

Also records what the producer factory's check now is. It is diagnostics, not
the guarantee: a producer made in a forked child refuses on first use anyway,
because it holds a `Weak<Runtime>` to the runtime the parent stamped and there
is no fresh stamp to make it look current. That was not true when each producer
copied a stamp at construction — a producer built in the child then took the
child's generation and never refused, which is why the old code had to block
that call. The refactor closed the bypass; the check is kept because failing at
`producer()` beats failing at the first `set()` across an FFI boundary, and it
now says so rather than implying it is load-bearing.

Three explicit checks remain, all in handle.rs and none guarding a resource:
the producer factory above, and the detach and Drop branches that decide to
release a thread rather than join one this process never had.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

Asked why the producer factory still checks explicitly, the honest answer
turned out to be the opposite of what the previous commit claimed. That commit
called it "diagnostics, not the guarantee" — reasoning that a producer built in
a child refuses on first use anyway, which is true but not the point.

Constructing a producer touches nothing gated: it only downgrades an `Arc`. So
without that line the call *succeeds* in a forked child, while `consumer()`
right below it refuses, because subscribing goes through `db()`. A handle that
hands out producers but not consumers is a worse contract than one that hands
out neither, and the difference stays invisible until the first publish. The
check is what makes the two factories agree, which is load-bearing enough.

It was also untested — the test covering it went when the fork suite was cut
from five tests to two. Rather than restore a binary, the two assertions join
`dropping_an_inherited_handle_does_not_panic`, which already has handles in the
child: no new fork, no new binary. Verified by removing the line and watching
that test fail, then restoring it.

The comment now says which of the two things it does: it is not what makes a
child safe, it is what stops the two factories disagreeing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

The shapes differ and the asymmetry deserves a reason in the code rather than
leaving the next reader to wonder whether the producer was simply missed.

Guarded exists to put a check in front of a resource nothing else gates. A
consumer owns one: its Reader is its own, and try_get reads straight out of it
without touching the runtime. A producer owns a key and a PhantomData. The only
resource it reaches is the database, which already sits behind Runtime::db —
private to that module, checked on the way through. Wrapping the key would mean
checking to reach a String and then checking again to reach the database: an
extra hop for no extra guarantee.

They also want opposite answers when the upgrade fails. A consumer must carry
on, because its buffer may still hold data — that is why RuntimeRef::check
returns Ok there, and why a detached consumer still delivers what was already
queued. A producer must refuse: there is nothing left to publish into. One
wrapper serving both would have to parameterise that policy, which costs more
than the hop it saves.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

Asked why `producer()` must call `check()` when there is nothing there to
guard, the honest answer is that it must not. The question exposed a
contradiction I had been defending rather than fixing: a check whose stated
justification was that it protects no resource is a check with no job.

Creating a producer touches nothing — not the database, not the runtime thread.
That is deliberate and pinned: `test_error_propagation` asserts an unregistered
key still yields a producer, and that `set()` reports the problem. A forked
child is one more thing `set()` reports, through the `db()` it has to pass. So
the check was making `fork` the single exception to this crate's own
lazy-producer contract.

It also left a category behind — "explicit checks that guard nothing" — and a
category is an invitation. Every check now guards a resource: `db()` and
`enter()` gate the database and the Tokio handle, `Guarded` gates the
consumer's reader, and the two in `detach`/`Drop` gate the `JoinHandle`,
where joining is the action and the check is a branch on state.

The fork test is updated to assert what is actually true and actually matters:
a child's `producer()` succeeds, and its first `set()` refuses. `consumer()`
still refuses outright, because subscribing goes through `db()` — an asymmetry
that predates this work, since an unregistered key already fails there and not
at `producer()`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

Removed at request. The reasoning it carried that the code still needs now
stands on its own where it is used — why the fork check lives on the way in,
why the consumer's reader is wrapped, and the 11-in-60 measurement that
explains why the fork suite is two tests rather than five. Those comments cited
the note by number; they no longer do, so nothing points at a file that is not
there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DKAH7JNjPLPmG4mPTvWfth

RuntimeRef inferred the fork check from a Weak<Runtime> upgrade and treated a
failed upgrade as "detached, let the read through". That conflates two things
which need opposite answers. A handle dropped in this process is a detach: the
buffer may still hold data and delivering it is behaviour the characterization
tests pin. A handle released in a forked child is not: the thread that fills
that buffer does not exist here. The Weak cannot tell them apart, and releasing
an inherited handle is precisely what a child is supposed to do.

So a child that dropped its handle passed the check. try_get reported
GetTimeout instead of ForkedChild, and get would park forever on the inherited
Tokio handle. Verified against both revisions with the same reproducer: main
reports ForkedChild, this branch reported GetTimeout. That is the silence the
generation check exists to prevent, reintroduced by the refactor meant to make
it unavoidable.

RuntimeRef now carries its runtime's generation, copied at construction by the
new Runtime::view - the only way to build one, and checked, so a view is not a
route around enter(). The Weak goes with it: once the stamp is carried, the
upgrade answers nothing, and liveness was never its job (a closed buffer is
what reports RuntimeShutdown).

Pinned three ways, since nothing covered RuntimeRef or Guarded before: unit
tests for the detach case, the fork case and both routes through Guarded,
against a stale-stamped view rather than a real fork; and the assertion folded
into the existing child that already drops a handle, so no third forking binary
and no new allocator-lock exposure. Mutating check() to the old semantics fails
all of them.

Also corrected four doc sites left describing the reverted Arc design - the rt
field stated the ownership invariant backwards, two cited keepalive clones that
producers and consumers do not hold, and one cited a RunningFlag that does not
exist - and the CHANGELOG's "no behaviour changed", which missed that
handle.producer() in a child now defers its refusal to set().

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BFpXuTKapm8XhFEGdMtCcr

The module note said producers and consumers hold a Weak. Consumers stopped
doing that when RuntimeRef began carrying its own generation, so the ownership
section now names the RuntimeRef and points at it for the reason.

The rest is length. "Why this is one type" and "Why the check lives on the way
in" argued the same point twice, so the history folds into the rule it
justifies. The RuntimeRef and Guarded notes both explained that a plain field
beside a hand-written guard is a call someone can forget; Guarded keeps it.
enter() and db() each spent a summary line and a blank on "the only way to
reach it". The test helper's 11-in-60 measurement stays, in one sentence.

108 comment lines, down from 141 and from the 122 this file carried before the
fix, which added a type and four tests. No fact dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BFpXuTKapm8XhFEGdMtCcr